Skip to content

fix(ui): implement smooth overflow scrolling and flex layout for mobi…#7953

Closed
Rahul123-dev-create wants to merge 1 commit into
ether:developfrom
Rahul123-dev-create:fix-mobile-toolbar-alignment
Closed

fix(ui): implement smooth overflow scrolling and flex layout for mobi…#7953
Rahul123-dev-create wants to merge 1 commit into
ether:developfrom
Rahul123-dev-create:fix-mobile-toolbar-alignment

Conversation

@Rahul123-dev-create

Copy link
Copy Markdown

Description

This PR resolves issue #7756 by updating the responsive CSS rules inside the Colibris skin toolbar component.

Changes Introduced

  • Switched the mobile .toolbar ul layouts to use flexbox configurations.
  • Added overflow-x: auto and optimized momentum scrolling (-webkit-overflow-scrolling: touch) for mobile containers to allow responsive horizontal swiping of overflowing styling tools.
  • Adjusted layout padding and margins to maintain touch-friendly tap targets across mobile viewports.

Fixes #7756

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used

Grey Divider


Remediation recommended

1. Missing final newline 🐞 Bug ⚙ Maintainability
Description
The modified toolbar.css ends without a trailing newline, violating the repository’s EditorConfig
and potentially failing formatting/lint checks in CI.
Code

src/static/skins/colibris/src/components/toolbar.css[R165-168]

}
.mobile-layout .toolbar ul li a:hover {
  /* background-color: transparent; */
-}
+}
Evidence
The repository requires a final newline for all files via EditorConfig, but this PR’s diff indicates
the CSS file now lacks it at EOF.

.editorconfig[1-9]
src/static/skins/colibris/src/components/toolbar.css[166-168]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/static/skins/colibris/src/components/toolbar.css` is missing the final newline at EOF, which violates `.editorconfig` (`insert_final_newline = true`) and may trigger formatting/lint failures.

## Issue Context
The PR diff explicitly shows `\ No newline at end of file`.

## Fix Focus Areas
- src/static/skins/colibris/src/components/toolbar.css[165-168]
- .editorconfig[1-9]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Wrap affects overflow detection 🐞 Bug ≡ Correctness
Description
Setting flex-wrap: wrap on .toolbar ul can eliminate horizontal overflow, changing
menuLeft.scrollWidth and preventing pad_editbar.ts from adding body.mobile-layout and/or
.toolbar.cropped in cases where the UI previously relied on those states to handle overflow.
Code

src/static/skins/colibris/src/components/toolbar.css[R17-20]

+  display: flex;
  align-items: center;
+  flex-wrap: wrap; /* Fixes layout crunching by allowing items to scale elegantly */
}
Evidence
The PR introduces wrapping on .toolbar ul, while the runtime logic that decides overflow handling
relies on menuLeft.scrollWidth thresholds; wrapping can make scrollWidth appear non-overflowing
even when the buttons no longer fit on a single line.

src/static/skins/colibris/src/components/toolbar.css[16-20]
src/static/js/pad_editbar.ts[370-386]
src/static/skins/colibris/src/components/toolbar.css[118-121]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pad_editbar.ts` decides whether to enable `body.mobile-layout` and `.toolbar.cropped` using `menuLeft.scrollWidth` comparisons. The PR adds `flex-wrap: wrap` to `.toolbar ul`, which can remove horizontal overflow by wrapping items to new lines, making `scrollWidth` no longer reflect “icons don’t fit on one row”. This can prevent `mobile-layout`/`cropped` from activating on some viewport/plugin combinations.

## Issue Context
`checkAllIconsAreDisplayedInToolbar()` explicitly removes `mobile-layout` before measuring, so the measurement happens under the base `.toolbar ul` rules (where `flex-wrap: wrap` now applies).

## Fix Focus Areas
- src/static/skins/colibris/src/components/toolbar.css[16-20]
- src/static/js/pad_editbar.ts[370-386]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Fix mobile toolbar overflow with flex layout and smooth horizontal scrolling
🐞 Bug fix 🕐 Less than 10 minutes

Grey Divider

Walkthroughs

Description
• Switch toolbar `` layout to flex to prevent mobile item crunching.
• Enable horizontal swipe scrolling on mobile while hiding scrollbars.
• Improve mobile tap targets and prevent icon shrinking for consistent alignment.
Diagram
graph TD
  U["Mobile user"] --> V["Mobile viewport"] --> L[".mobile-layout"] --> C["toolbar.css"] --> UL[".toolbar ul"] --> S["Overflow scroll"]
  UL --> LI["Toolbar items"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use scroll-snap for deterministic button paging
  • ➕ Improves discoverability by snapping to button groups on swipe
  • ➕ Can make overflow navigation feel more intentional than free scrolling
  • ➖ Adds more CSS complexity and requires careful tuning across browsers
  • ➖ May feel restrictive compared to natural free scrolling for toolbars
2. Keep wrapping but add responsive grouping/overflow menu
  • ➕ Avoids horizontal scrolling entirely on very small viewports
  • ➕ Can reduce visual clutter by collapsing rarely used tools
  • ➖ Higher implementation complexity (requires HTML/JS behavior, not just CSS)
  • ➖ Changes interaction model and may regress muscle memory

Recommendation: Current approach (flex + overflow-x auto + momentum scrolling) is the lowest-risk, most native-feeling fix for #7756 and keeps behavior purely CSS-driven. Consider scroll-snap only if usability testing shows users miss off-screen tools.

Grey Divider

File Changes

Bug fix (1)
toolbar.css Enable mobile flex toolbar with horizontal overflow scrolling +16/-2

Enable mobile flex toolbar with horizontal overflow scrolling

• Converts the toolbar '<ul>' to a flex container and adds mobile-only horizontal overflow scrolling with momentum ('-webkit-overflow-scrolling: touch') while hiding scrollbars. Adjusts mobile list-item spacing and prevents flex shrink to keep tap targets usable and icons aligned. File also ends without a trailing newline.

src/static/skins/colibris/src/components/toolbar.css


Grey Divider

Qodo Logo

@JohnMcLear

Copy link
Copy Markdown
Member

I think you can see what is wrong with this PR.

@JohnMcLear JohnMcLear closed this Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance: Look deeper into scaling N+ contributors per pad.

2 participants